풀이 추가/수정 커밋시 기존 분석은 유지하고 추가/수정 파일에 대해서만 분석하도록 변경 - #50
Conversation
기존에는 풀이를 추가하거나 수정해서 커밋하면 기존 분석 댓글을 모두 삭제하고 모든 풀이에 대한 분석 댓글을 다시 달았음. 분석 댓글에서 대화를 주고 받는 경우가 있는데 커밋이 되면 분석 내용이 삭제되고 주고 받은 댓글 일부가 Conversations 탭에서 사라지는 문제가 있었음. 변경된 방식은 풀이를 추가하거나 수정해도 기존 분석 댓글을 유지함. 추가되거나 수정된 풀이에 대해서만 분석하여 댓글을 남김. 분석했던 코드 원본을 함께 분석 댓글에 포함하여 맥락을 파악하는데 용이하도록 함. 기존과 동일한 부분 - PR 오픈과 재오픈한 경우엔 모든 풀이에 대해서 분석 댓글을 남김. - 삭제된 풀이는 분석하지 않음. Test: bun run test Issue: DaleStudy#48
| async function resolveChangedFilenames(payload, repoOwner, repoName, appToken) { | ||
| if (payload.action !== "synchronize" || !payload.before || !payload.after) { | ||
| return null; | ||
| } | ||
|
|
||
| let changedFilenames = null; | ||
|
|
||
| try { | ||
| changedFilenames = await getChangedFilenames( | ||
| repoOwner, | ||
| repoName, | ||
| payload.before, | ||
| payload.after, | ||
| appToken | ||
| ); | ||
| } catch (error) { | ||
| console.error(`[resolveChangedFilenames] failed: ${error.message}`); | ||
| } | ||
|
|
||
| return changedFilenames; | ||
| } |
There was a problem hiding this comment.
일반 push는 before가 after의 조상이라 정확하지만, 충돌 해결 등으로 rebase 후 강제 푸시하면 merge base가 브랜치 분기점까지 내려가서 PR의 모든 풀이 파일이 changedFilenames에 포함도지 않을까요? 삭제 로직이 있을 때는
지우고 다시 다니 상관없었는데 이제는 전 파일에 댓글이 한 벌씩 더 쌓이게 될텐데 새로운 엣지 케이스가 우려가 되네요.
There was a problem hiding this comment.
말씀대로 rebase 후 강제 푸시하면 변경이 없더라도 모든 파일이 changedFilenames에 포함되는 문제가 있네요. 정확하게 파일 내용이 변경된 경우에만 분석을 다시 하도록 파일 SHA 이용하는 방식으로 변경해보았어요.
다음 PR에서 테스트 시나리오 대로 테스트 해두었고 문제가 되었던 rebase 후 강제 푸시하는 경우도 포함했습니다.
테스트 시나리오
1. 초기 3개 파일로 PR을 연다.
2. 2개 파일을 추가해서 커밋한다.
3. 기존 파일 3개를 수정한 뒤 리베이스하고 force-push한다.
4. PR을 닫았다가 다시 연다.
5. 기존 파일 2개를 수정해서 커밋한다.
| let body = `${COMMENT_MARKER} | ||
| ### 🏷️ 알고리즘 패턴 분석 | ||
|
|
||
| ${renderAnalyzedSource(file.filename, fileContent)} |
There was a problem hiding this comment.
원본 소스코드를 보여주는 아이디어 좋네요!
코드 변경 이후의 히스토리 저장 측면에서도 유용하다고 생각합니다.
다만 여기서 더 나아가 diff를 보여준다던지 하는 건 없을까요? 어디서 어떻게 변경됐는지도 알 수 있으면 좋겠다 생각했기 떄문이에요.
그러면 깃허브에서 자동으로 제공하는 diff 링크 같은 게 있을까도 궁금합니다.
아래 코멘트에서 언급된 rebase로 인한 force push 업데이트에서도 깃허브에서 변경사항을 보여주는 링크가 제공되기에 해결할 수 있는 방법 중 하나가 되지 않을까 생각해봤습니다.
There was a problem hiding this comment.
@sounmind님 안녕하세요 좋은 제안 감사합니다.
diff를 보여주는 거 좋은 거 같고 이건 후속 PR로 올려보도록 하겠습니다.
downloadFileEntries가 isContentTruncated 값을 함께 반환하고, 해당 값을 분석 댓글 렌더링에 사용하도록 변경한다. MAX_FILE_SIZE는 파일의 바이트 단위 크기로 오해할 수 있어 문자열 길이 제한임이 드러나도록 MAX_FILE_CONTENT_LENGTH로 변경한다. 생략 문구 표시에 대한 경계값 테스트를 추가한다.
27edff1 to
f99b314
Compare
파일 내용에 포함된 가장 긴 연속 백틱보다 긴 fence를 생성하여 분석 댓글의 코드 블록이 중간에 닫히지 않도록 한다.
분석 댓글에 파일 SHA를 기록하고 기존 분석 댓글의 SHA와 비교해 아직 분석하지 않았거나 파일 내용이 변경된 풀이만 분석한다. 웹훅의 changedFilenames 계산 및 전달 로직을 제거한다.
331e11b to
f6a09d3
Compare
3 participants
문제
커밋으로 풀이가 수정/추가되면 기존 분석 댓글을 모두 삭제하고 모든 파일에 대한 분석 댓글을 다시 달고 있음.
분석 댓글에서 대화를 주고 받는 경우가 있는데 커밋이 되면 분석 내용이 삭제되고 주고 받은 댓글 일부가
Conversations 탭에서 사라짐.
변경
풀이가 수정/추가되어도 기존 분석 댓글을 삭제하지 않고 변경된 파일에 대해서만 분석하여 댓글을 남김.
분석했던 코드 원본을 함께 댓글에 포함하여 맥락을 파악하는데 용이하도록 함.
이 pr에서 스크린샷의 동작 결과는 살펴보실 수 있습니다.
유지
고민사항
수정이 많아지면 계속 댓글이 추가되어 길어질 수 있음. 현재 기수의 경우 수정하는 경우가 수 회 정도라 수용가능한 정도로 보임.
테스트
Test: bun run test
cloudflare 개인 계정을 만들고 organization Dalestudy-test을 만들어서 테스트함.
Close: #48